Auto-sorting core style #1000
Auto-sorting core style #1000
Conversation
WalkthroughAdds Prettier JSON-sorting pre-commit hook and config, expands Prettier ignore patterns, restructures Plugwise icons/strings/translations, centralizes venv/uv/setup flow and adds ulimit/prettierrc validation in core-testing.sh, and re-enables Ruff rule UP038 in pyproject.toml. Changes
Sequence Diagram(s)sequenceDiagram
actor Developer
participant Script as core-testing.sh
participant Venv as VirtualEnv
participant UV as uv (task runner)
participant Setup as script/setup
participant Prettier as Prettier validation
Developer->>Script: invoke core-testing.sh
Script->>Venv: ensure/create venv (ulimit set)
Script->>UV: ensure uv installed/available in venv
UV->>Venv: install/run pytest & tools if needed
Script->>Setup: run script/setup to finalize env and install module from manifest
Script->>Prettier: validate/sync .prettierrc.js and .prettierignore
Script->>Developer: environment ready / proceed with tests
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
CoMPaTech
added
enhancement
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
custom_components/plugwise/strings.json (1)
40-66: Missinglow_batterybinary sensor in source strings.The
low_batterybinary sensor is missing from the sourcestrings.jsonbut present innl.json. Per learnings, this entity (defined asBATTERY_STATEinconst.py) should be included. Sincestrings.jsonis the source file for translations, adding it here will ensure consistency across all translation files.Proposed fix to add missing translation
"heating_state": { "name": "Heating" }, + "low_battery": { + "name": "Low battery" + }, "plugwise_notification": { "name": "Plugwise notification" },
🤖 Fix all issues with AI agents
In @.pre-commit-config.yaml:
- Around line 61-68: Update the Prettier hook to a patched release by changing
the repo rev and the additional_dependencies entry that currently reference
v3.6.2/[email protected] to a fixed version (e.g., bump both rev and prettier@ to a
patched release >=3.6.3 or the latest stable release); target the hook with id
"prettier" and the additional_dependencies line "[email protected]" so both are
updated together to the chosen patched version.
In @.prettierrc.js:
- Around line 1-24: The jsonSortOrder values are using computed RegExp keys
(e.g., [/.*/]) which become non-serializable objects when passed to
JSON.stringify; update both occurrences so the regex keys are plain strings
(e.g., use "/.*/" instead of [/.*/]) inside the objects used by jsonSortOrder so
JSON.stringify({ "/.*/": "numeric" }) is produced; keep the existing
JSON.stringify(...) calls and only change the property keys to string literals
in the jsonSortOrder entries.
In @custom_components/plugwise/translations/en.json:
- Around line 40-66: The English translations are missing the low_battery binary
sensor entry; add a "low_battery" key under "entity" -> "binary_sensor" with an
appropriate name (e.g., "Low battery") so the English file matches nl.json and
other locales; this corresponds to the BATTERY_STATE binary sensor defined in
const.py and should mirror the existing pattern used by other sensors like
"flame_state" and "heating_state".
- Around line 67-71: Add the missing button.reboot translation entry to nl.json
to match en.json/strings.json; specifically add a "button" object containing
"reboot": {"name": "<Dutch translation>"} (e.g., "Herstarten") so the
key/button.reboot exists in nl.json with the Dutch label.
In @custom_components/plugwise/translations/nl.json:
- Around line 227-229: The translation key heating_setpoint has a leading space
in its "name" value; remove the leading space so the string is "Instelling
verwarmen" (update the "heating_setpoint" -> "name" value to eliminate the
initial whitespace).
- Around line 68-70: Add the missing "button" translation block to the Dutch
translations by inserting a "button" object with the "reboot" key and its Dutch
label (matching the structure used in en.json/strings.json); update the nl.json
JSON structure so it contains "button": { "reboot": "<Dutch label>" } at the
same level as "climate" to keep format and keys consistent with the other locale
files.
🧹 Nitpick comments (1)
scripts/core-testing.sh (1)
68-79: Prefer explicit venv directory checks.The venv setup logic at the script's beginning creates a venv if
$VIRTUAL_ENVis not set, but later at line 166 the script checksif [ ! -d "venv" ]. Consider using consistent checks to avoid creating multiple virtual environments.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.pre-commit-config.yaml.prettierignore.prettierrc.jscustom_components/plugwise/icons.jsoncustom_components/plugwise/strings.jsoncustom_components/plugwise/translations/en.jsoncustom_components/plugwise/translations/nl.jsonscripts/core-testing.sh
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: CoMPaTech
Repo: plugwise/plugwise-beta PR: 0
File: :0-0
Timestamp: 2025-01-25T20:42:24.862Z
Learning: The linting rules in plugwise-beta have been updated to use TC001/TC002/TC003 instead of TCH001/TCH002/TCH003 for type-checking related rules.
Learnt from: CoMPaTech
Repo: plugwise/plugwise-beta PR: 789
File: .pre-commit-config.yaml:3-3
Timestamp: 2024-12-08T11:23:47.692Z
Learning: CoMPaTech considers Python 3.13 stable and prefers to use it in the project.
📚 Learning: 2025-01-25T20:42:24.862Z
Learnt from: CoMPaTech
Repo: plugwise/plugwise-beta PR: 0
File: :0-0
Timestamp: 2025-01-25T20:42:24.862Z
Learning: The linting rules in plugwise-beta have been updated to use TC001/TC002/TC003 instead of TCH001/TCH002/TCH003 for type-checking related rules.
Applied to files:
.prettierignore
📚 Learning: 2025-01-25T20:42:24.862Z
Learnt from: CoMPaTech
Repo: plugwise/plugwise-beta PR: 0
File: :0-0
Timestamp: 2025-01-25T20:42:24.862Z
Learning: In the plugwise-beta project, `uv` package manager was removed from core-testing.sh in favor of pip due to issues it was causing.
Applied to files:
scripts/core-testing.sh
📚 Learning: 2026-01-07T19:13:26.648Z
Learnt from: bouwew
Repo: plugwise/plugwise-beta PR: 998
File: custom_components/plugwise/translations/nl.json:60-62
Timestamp: 2026-01-07T19:13:26.648Z
Learning: In all Plugwise translation files (e.g., custom_components/plugwise/translations/*.json), ensure the low_battery binary_sensor (defined as BATTERY_STATE in const.py) is included with the correct Dutch label. This keeps translations in sync with available entities and avoids missing translations.
Applied to files:
custom_components/plugwise/translations/nl.jsoncustom_components/plugwise/translations/en.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Prepare and validate pre-commit
- GitHub Check: Prepare and validate pre-commit
- GitHub Check: Prepare and validate pre-commit
🔇 Additional comments (9)
.prettierignore (1)
1-5: LGTM!The expanded ignore patterns appropriately focus Prettier on JSON files while excluding markdown, YAML, tests, and GitHub workflows. Self-ignoring the configuration file is good practice.
custom_components/plugwise/icons.json (2)
69-73: LGTM!The new icon mappings for
dhw_modestates are semantically appropriate:
comfort→mdi:sofaeco→mdi:leafThe existing icons for
auto,boost, andoffmaintain consistency with Material Design patterns.
87-91: LGTM!The addition of the
coolingstate withmdi:snowflakeicon is appropriate and aligns well with the existing heating-related icons. The icon choices maintain visual consistency across regulation modes.scripts/core-testing.sh (3)
30-31: LGTM!The ulimit increase to 65536 file descriptors aligns with modern Python tooling requirements, particularly for uv and Astral tools. This prevents file descriptor exhaustion during package operations.
166-178: Verify the reintroduction of uv.The script now prefers
uvfor venv creation when available, falling back to standardvenv. However, according to project learnings, uv was previously removed from this script due to issues it was causing.Ensure that the issues that led to uv's removal have been resolved, or clarify why it's being reintroduced in this limited capacity (venv creation only).
Based on learnings, uv was previously problematic in this script.
191-196: LGTM!The prettierrc validation ensures that changes to the core Home Assistant Prettier configuration are properly propagated to the plugwise-beta repository. The
sedtransformation correctly adjusts paths fromhomeassistanttocustom_components.custom_components/plugwise/strings.json (1)
1-345: Structure and content look good overall.The reorganization to a top-level
configobject with nestedabort,error, andstepsections follows Home Assistant's integration translation conventions. The alphabetical sorting (from Prettier) improves maintainability.custom_components/plugwise/translations/nl.json (2)
60-62:low_batteryis correctly included here.This translation file correctly includes the
low_batterybinary sensor. However, this entity is missing fromen.jsonandstrings.json- those files should be updated to match.
1-343: Dutch translations are comprehensive and well-structured.The reorganization aligns with the English translations, and the Dutch text is appropriately localized. The new
optionssection andselect_gateway_modeentity have proper Dutch translations.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
scripts/core-testing.sh (2)
30-32: Consider clarifying the comment.The term "astral-uv" in the comment is non-standard. Consider rephrasing to "core and uv expectations" or "uv expectations" for clarity.
📝 Suggested comment clarification
-# Ensure ulimit is setup to match core/astral-uv expectations +# Ensure ulimit is setup to match core and uv expectations ulimit -n 65536
171-178: Consider consistent indentation.These lines use space indentation (8 spaces) while other parts of the script may use different indentation styles. For consistency, consider aligning with the project's shell script indentation convention.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/core-testing.sh
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CoMPaTech
Repo: plugwise/plugwise-beta PR: 0
File: :0-0
Timestamp: 2025-01-25T20:42:24.862Z
Learning: The linting rules in plugwise-beta have been updated to use TC001/TC002/TC003 instead of TCH001/TCH002/TCH003 for type-checking related rules.
Learnt from: CoMPaTech
Repo: plugwise/plugwise-beta PR: 789
File: .pre-commit-config.yaml:3-3
Timestamp: 2024-12-08T11:23:47.692Z
Learning: CoMPaTech considers Python 3.13 stable and prefers to use it in the project.
📚 Learning: 2025-01-25T20:42:24.862Z
Learnt from: CoMPaTech
Repo: plugwise/plugwise-beta PR: 0
File: :0-0
Timestamp: 2025-01-25T20:42:24.862Z
Learning: In the plugwise-beta project, `uv` package manager was removed from core-testing.sh in favor of pip due to issues it was causing.
Applied to files:
scripts/core-testing.sh
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Prepare and validate pre-commit
🔇 Additional comments (3)
scripts/core-testing.sh (3)
190-197: LGTM! Prettierrc sync logic is correct.The validation correctly ensures that the project's
.prettierrc.jsstays synchronized with HA core's configuration, with appropriate path transformations. This aligns well with the PR's objective of adopting core's auto-sorting style.
213-217: The current implementation correctly extracts and installs the requirement from manifest.json. The manifest currently contains a single requirement (plugwise==1.11.2), and the code handles it properly. If multiple requirements are added to the manifest in the future, the code would pass all of them touv pip installwithout issue.
171-178: No actionable issues. The uv integration uses a proper fallback pattern that gracefully handles missing dependencies. The most recent refinement commit indicates this approach is working as intended.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.pre-commit-config.yaml:
- Line 70: The pre-commit hook revision for the Biome hook was changed to "rev:
v2.3.10" from v2.3.11; confirm whether this downgrade is intentional and either
(a) restore the intended revision value (e.g., set "rev" back to v2.3.11) if the
downgrade was accidental, or (b) add a brief comment in the hook configuration
or PR description explaining the intentional rollback (e.g., compatibility/bug
workaround for v2.3.11) so reviewers understand the reason; reference the "rev:
v2.3.10" line and the previously used v2.3.11 in your change.
- Line 55: The pre-commit config references a non-existent shellcheck-py tag
("rev: v0.11.0.1"); update that rev to a valid tag from the shellcheck-py
repository (or replace with the latest stable tag) so the hook can be fetched
successfully, and also resolve the version mismatches: restore biome to the
intended v2.3.11 (remove the accidental downgrade to v2.3.10) and make the
prettier pin consistent by aligning the mirror and additional_dependencies to
the same version (e.g., change additional_dependencies to [email protected] or
change the mirror to 3.6.2) so all three entries (shellcheck-py rev, biome, and
prettier mirror/additional_dependencies) reference valid, matching versions.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.pre-commit-config.yaml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CoMPaTech
Repo: plugwise/plugwise-beta PR: 0
File: :0-0
Timestamp: 2025-01-25T20:42:24.862Z
Learning: The linting rules in plugwise-beta have been updated to use TC001/TC002/TC003 instead of TCH001/TCH002/TCH003 for type-checking related rules.
Learnt from: CoMPaTech
Repo: plugwise/plugwise-beta PR: 789
File: .pre-commit-config.yaml:3-3
Timestamp: 2024-12-08T11:23:47.692Z
Learning: CoMPaTech considers Python 3.13 stable and prefers to use it in the project.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Setup for HA-core (release/master)
- GitHub Check: Setup for HA-core (dev)
🔇 Additional comments (2)
.pre-commit-config.yaml (2)
74-74: Good clarification of the hook's purpose.The updated name accurately reflects that this hook runs biome, not spelling checks. The previous name appeared to be incorrectly copied from the codespell hook.
61-68: No action required. The prettier configuration uses compatible versions: [email protected] is a valid release and satisfies the peerDependencies requirement of [email protected] (which requires prettier >=3). Using a specific version (3.6.2) rather than the mirror repo's latest tag (v3.7.4) is a normal and intentional pattern for pre-commit hooks.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
custom_components/plugwise/translations/nl.json (2)
16-16: Typo in Dutch translation: "Initaliseren" should be "Initialiseren".Missing letter 'i' in the Dutch word.
🔤 Suggested fix
- "stick_init": "Initaliseren van USB-stick mislukt", + "stick_init": "Initialiseren van USB-stick mislukt",
14-14: Mixed Dutch/English: "Network" should be "Netwerk".The word "Network" is English while the rest of the message is Dutch.
🔤 Suggested fix
- "network_timeout": "Network communicatie timeout", + "network_timeout": "Netwerk communicatie timeout",
🤖 Fix all issues with AI agents
In @custom_components/plugwise/translations/nl.json:
- Around line 20-37: The translation uses inconsistent terminology for port:
change the value for key step.reconfigure.data.port ("Port nummer") to the
correct and consistent Dutch term "Poortnummer" to match step.user.data.port and
keep translations uniform.
In @scripts/core-testing.sh:
- Around line 62-69: The venv_and_uv function blindly sources venv/bin/activate
which breaks when the virtualenv isn't present; add a guard that checks for the
activate script (e.g., test -f "venv/bin/activate" or
"${coredir}/venv/bin/activate") before sourcing and, if missing, print a clear
error and return a non-zero status (or create the venv if desired) to avoid an
uncaught failure; then continue with the existing uv check/installation. Ensure
you update the venv_and_uv function name and the exact path used in the source
call so they match.
🧹 Nitpick comments (2)
custom_components/plugwise/translations/en.json (1)
1-348: Translation file structure looks correct.The English translation file is properly structured with all required sections (config, entity, exceptions, options, services) and includes the
low_batterybinary_sensor as expected. Based on learnings, this aligns with the requirement to keep translations in sync with available entities.One minor branding note on line 330: "Homekit" should be "HomeKit" (capital K) to match Apple's official branding.
🔤 Suggested fix
- "homekit_emulation": "Homekit emulation (i.e. on hvac_off => Away) *) beta-only option", + "homekit_emulation": "HomeKit emulation (i.e. on hvac_off => Away) *) beta-only option",custom_components/plugwise/strings.json (1)
1-348: Strings file is consistent with en.json.The source strings file structure and content align correctly with the English translation file. All entity keys, error messages, and configuration options are properly defined.
Same minor branding suggestion as en.json: line 330 should use "HomeKit" instead of "Homekit".
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
custom_components/plugwise/strings.jsoncustom_components/plugwise/translations/en.jsoncustom_components/plugwise/translations/nl.jsonpyproject.tomlscripts/core-testing.sh
💤 Files with no reviewable changes (1)
- pyproject.toml
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: CoMPaTech
Repo: plugwise/plugwise-beta PR: 0
File: :0-0
Timestamp: 2025-01-25T20:42:24.862Z
Learning: The linting rules in plugwise-beta have been updated to use TC001/TC002/TC003 instead of TCH001/TCH002/TCH003 for type-checking related rules.
Learnt from: CoMPaTech
Repo: plugwise/plugwise-beta PR: 789
File: .pre-commit-config.yaml:3-3
Timestamp: 2024-12-08T11:23:47.692Z
Learning: CoMPaTech considers Python 3.13 stable and prefers to use it in the project.
📚 Learning: 2026-01-07T19:13:26.648Z
Learnt from: bouwew
Repo: plugwise/plugwise-beta PR: 998
File: custom_components/plugwise/translations/nl.json:60-62
Timestamp: 2026-01-07T19:13:26.648Z
Learning: In all Plugwise translation files (e.g., custom_components/plugwise/translations/*.json), ensure the low_battery binary_sensor (defined as BATTERY_STATE in const.py) is included with the correct Dutch label. This keeps translations in sync with available entities and avoids missing translations.
Applied to files:
custom_components/plugwise/translations/nl.jsoncustom_components/plugwise/translations/en.json
📚 Learning: 2025-01-25T20:42:24.862Z
Learnt from: CoMPaTech
Repo: plugwise/plugwise-beta PR: 0
File: :0-0
Timestamp: 2025-01-25T20:42:24.862Z
Learning: The linting rules in plugwise-beta have been updated to use TC001/TC002/TC003 instead of TCH001/TCH002/TCH003 for type-checking related rules.
Applied to files:
custom_components/plugwise/translations/en.json
📚 Learning: 2026-01-07T19:13:34.937Z
Learnt from: bouwew
Repo: plugwise/plugwise-beta PR: 998
File: custom_components/plugwise/translations/nl.json:60-62
Timestamp: 2026-01-07T19:13:34.937Z
Learning: The low_battery binary_sensor (defined as BATTERY_STATE in custom_components/plugwise/const.py) is a valid entity in the Plugwise integration and should be included in translation files.
Applied to files:
custom_components/plugwise/strings.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Prepare and validate pre-commit
🔇 Additional comments (4)
custom_components/plugwise/translations/nl.json (1)
60-62: Low battery translation is present as required.The
low_batterybinary_sensor is correctly included with Dutch translation "Batterij status", keeping translations in sync with available entities. Based on learnings, this is a required translation key.scripts/core-testing.sh (3)
30-32: LGTM: Appropriate ulimit configuration.The ulimit setting ensures sufficient file descriptors for uv and core operations, which is especially important for dependency resolution and parallel operations.
175-182: LGTM: Proper venv initialization flow.The logic correctly separates venv creation from environment setup via
script/setup, which aligns with HA core's recommended setup process. The re-activation afterscript/setupensures the venv is properly configured.
194-201: LGTM: Excellent automation for config synchronization.The prettierrc validation ensures formatting rules stay synchronized with HA core, automatically updating when core's config changes. The path transformation (
homeassistant→custom_components) is appropriate for the custom component structure.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @scripts/core-testing.sh:
- Around line 198-203: The variable expansions for prettierrc are unquoted;
change all occurrences of ${prettierrc} in the diff and sed calls to quoted
expansions (e.g., "${prettierrc}" and "../${prettierrc}") so the process
substitution and redirection use a single, safe token; update both the diff -q
<(sed 's/homeassistant/custom_components/g' "${prettierrc}") "../${prettierrc}"
and the sed 's/homeassistant/custom_components/g' ${prettierrc} >
../${prettierrc} usages to use quoted "${prettierrc}" forms.
🧹 Nitpick comments (1)
scripts/core-testing.sh (1)
62-72: Add defensive check for venv existenceThe function sources
venv/bin/activatewithout verifying the venv exists. While the current script flow ensures venv creation before calling this function, adding a defensive check would improve robustness and reusability.♻️ Proposed defensive check
venv_and_uv() { + if [ ! -d "venv" ]; then + echo -e "${CFAIL}venv directory not found${CNORM}" + return 1 + fi # shellcheck disable=SC1091 source venv/bin/activate if ! [ -x "$(command -v uv)" ]; then
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/core-testing.sh
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CoMPaTech
Repo: plugwise/plugwise-beta PR: 0
File: :0-0
Timestamp: 2025-01-25T20:42:24.862Z
Learning: The linting rules in plugwise-beta have been updated to use TC001/TC002/TC003 instead of TCH001/TCH002/TCH003 for type-checking related rules.
Learnt from: CoMPaTech
Repo: plugwise/plugwise-beta PR: 789
File: .pre-commit-config.yaml:3-3
Timestamp: 2024-12-08T11:23:47.692Z
Learning: CoMPaTech considers Python 3.13 stable and prefers to use it in the project.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Prepare and validate pre-commit
🔇 Additional comments (5)
scripts/core-testing.sh (5)
30-32: LGTM - ulimit setting for file descriptorsSetting the file descriptor limit early in the script is appropriate for ensuring compatibility with HA-core and uv expectations.
178-185: LGTM - consolidated venv setup flowThe venv creation and setup flow is well-structured: checks for venv existence, creates if needed using uv, runs setup script, and activates the environment.
212-212: LGTM - venv_and_uv usage in pip_prepThe venv_and_uv call is appropriately placed, as core_prep ensures venv existence before this stage.
231-231: LGTM - consistent venv_and_uv usageThe venv_and_uv calls in the testing and quality sections follow the same safe pattern as pip_prep.
Also applies to: 263-263
286-286: LGTM - final venv_and_uv usageThe venv_and_uv call for the copy-back operations is consistent with other usages throughout the script.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @scripts/core-testing.sh:
- Around line 62-75: The venv_and_uv function currently uses a fragile check `uv
pip list | grep -q bcrypt` to decide whether to run `script/setup`; replace this
with a robust readiness check: have `script/setup` write a marker file (e.g.,
.env_ready) or check for a clear, single package like `homeassistant`, or verify
multiple required packages exist; update venv_and_uv to test for that marker
file (or multiple packages) before invoking `script/setup`, and keep the
existing checks for `uv` and `pytest` intact so the function reliably detects a
completed environment instead of relying on presence of `bcrypt`.
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
55-60: Consider unique step names for better log clarity.Both steps share the name "Core dependencies", which may make workflow logs harder to parse. Consider naming them distinctly (e.g., "Update apt cache" and "Install Core system dependencies").
♻️ Suggested improvement
- - name: Core dependencies - id: core-dependencies-1 - run: sudo apt-get update - - name: Core dependencies - id: core-dependencies-2 - run: sudo apt-get install python3-pip python3-dev python3-venv autoconf libssl-dev libxml2-dev libxslt1-dev libjpeg-dev libffi-dev libudev-dev zlib1g-dev pkg-config libavformat-dev libavcodec-dev libavdevice-dev libavutil-dev libswscale-dev libswresample-dev libavfilter-dev ffmpeg libgammu-dev build-essential + - name: Update apt cache + id: core-dependencies-1 + run: sudo apt-get update + - name: Install Core system dependencies + id: core-dependencies-2 + run: | + sudo apt-get install \ + python3-pip python3-dev python3-venv \ + autoconf libssl-dev libxml2-dev libxslt1-dev \ + libjpeg-dev libffi-dev libudev-dev zlib1g-dev \ + pkg-config libavformat-dev libavcodec-dev \ + libavdevice-dev libavutil-dev libswscale-dev \ + libswresample-dev libavfilter-dev ffmpeg \ + libgammu-dev build-essential
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/test.ymlcustom_components/plugwise/translations/nl.jsonscripts/core-testing.sh
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CoMPaTech
Repo: plugwise/plugwise-beta PR: 0
File: :0-0
Timestamp: 2025-01-25T20:42:24.862Z
Learning: The linting rules in plugwise-beta have been updated to use TC001/TC002/TC003 instead of TCH001/TCH002/TCH003 for type-checking related rules.
Learnt from: CoMPaTech
Repo: plugwise/plugwise-beta PR: 789
File: .pre-commit-config.yaml:3-3
Timestamp: 2024-12-08T11:23:47.692Z
Learning: CoMPaTech considers Python 3.13 stable and prefers to use it in the project.
📚 Learning: 2026-01-07T19:13:26.648Z
Learnt from: bouwew
Repo: plugwise/plugwise-beta PR: 998
File: custom_components/plugwise/translations/nl.json:60-62
Timestamp: 2026-01-07T19:13:26.648Z
Learning: In all Plugwise translation files (e.g., custom_components/plugwise/translations/*.json), ensure the low_battery binary_sensor (defined as BATTERY_STATE in const.py) is included with the correct Dutch label. This keeps translations in sync with available entities and avoids missing translations.
Applied to files:
custom_components/plugwise/translations/nl.json
📚 Learning: 2025-01-25T20:42:24.862Z
Learnt from: CoMPaTech
Repo: plugwise/plugwise-beta PR: 0
File: :0-0
Timestamp: 2025-01-25T20:42:24.862Z
Learning: The linting rules in plugwise-beta have been updated to use TC001/TC002/TC003 instead of TCH001/TCH002/TCH003 for type-checking related rules.
Applied to files:
custom_components/plugwise/translations/nl.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Setup for HA-core (dev)
- GitHub Check: Setup for HA-core (release/master)
🔇 Additional comments (12)
custom_components/plugwise/translations/nl.json (8)
3-8: LGTM: Config abort messages properly structured.The new nested
config.abortstructure is well-organized with appropriate Dutch translations for abort scenarios.
9-19: LGTM: Comprehensive error message translations.All error scenarios are properly translated into Dutch with clear, user-friendly messages.
20-38: LGTM: Config flow steps well-structured.Both
reconfigureandusersteps contain complete Dutch translations for all data fields and descriptions.
60-62: Low battery sensor included as expected.The
low_batterybinary sensor is properly included with the Dutch label "Batterij status", consistent with the requirement to keep translations in sync with available entities.Based on learnings, this sensor (defined as BATTERY_STATE in const.py) should be present in all Plugwise translation files.
299-323: LGTM: Exception messages properly translated.All exception messages include appropriate Dutch translations with correct placeholder syntax where needed (e.g.,
{error}).
325-341: LGTM: Options flow properly structured.The new
optionssection correctly handles both scenarios:
initstep with configurable options (including beta-only features clearly marked)nonestep for when no options are availableAll Dutch translations are complete and descriptive.
342-347: LGTM: Service translation complete.The
delete_notificationservice has both description and name properly translated. The field ordering (description before name) appears to be from the auto-sorting applied as mentioned in the PR title.
48-50: No action required. All entity translation keys correspond to actual definitions in the codebase. Verification confirms:
cooling_state(const.py:36)secondary_boiler_state(const.py:41)rebootbutton (const.py:23)maximum_boiler_temperature(const.py:79)select_gateway_mode(const.py:138, used in select.py)- All
electricity_*andvoltage_*sensors (const.py:89-127)These translations are not unused and properly support existing entities in the integration.
scripts/core-testing.sh (4)
30-31: Verify ulimit behavior in restricted environments.The
ulimit -n 65536command may fail in containerized or restricted environments where users lack permission to modify limits. Sinceset -eis active, this would abort the script.Consider making this more resilient:
♻️ Suggested safeguard
-# Ensure ulimit is setup to match core/astral-uv expectations -ulimit -n 65536 +# Ensure ulimit is setup to match core/astral-uv expectations +ulimit -n 65536 || echo -e "${CWARN}Could not set ulimit (may be restricted in your environment)${CNORM}"
181-188: LGTM!The initialization flow is clear: ensure venv exists, run setup, and activate. The messaging helps users understand the preparation steps.
215-215: LGTM!The centralized
venv_and_uvhelper improves maintainability by consolidating venv/uv setup logic. The consistent usage across different sections is good practice.Also applies to: 234-234, 266-266, 289-289
201-207: No issue found with sed replacement in prettierrc synchronization.The
.prettierrc.jsfile contains onlycustom_componentsreferences and no instances of "homeassistant" text. Since the sed pattern's/homeassistant/custom_components/g'has no matching text to replace, the command is harmless and produces no unintended side effects.Likely an incorrect or invalid review comment.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Also leverages core's use of uv for all packages instead of cherry-picking
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.